Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance Resource Constraints Validation Policy to Include InitContainers & EphermalContainers #1314

Conversation

nikhilmaheshwari24
Copy link
Contributor

@nikhilmaheshwari24 nikhilmaheshwari24 commented Jul 30, 2024

Signed-off-by: nikhilmaheshwari24 nikhil.m2498@gmail.com

Related issue

kyverno/policies#951

Proposed Changes

This PR updates the existing Kyverno policy to validate CPU and memory resource requests and limits for both containers and initContainers within Pods. This enhancement ensures comprehensive resource management across all container types in a Kubernetes cluster.

Checklist

  • I have read the contributing guidelines.
  • I have inspected the website preview for accuracy.
  • I have signed off on my issue.

Signed-off-by: Nikhil Maheshwari <36232275+nikhilmaheshwari24@users.noreply.github.com>
@nikhilmaheshwari24
Copy link
Contributor Author

Hi, @realshuting

Please review & help with your approval. TIA

Copy link
Member

@realshuting realshuting left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about ephemeralContainers?

@realshuting realshuting self-assigned this Jul 30, 2024
Signed-off-by: Nikhil Maheshwari <36232275+nikhilmaheshwari24@users.noreply.github.com>
@nikhilmaheshwari24
Copy link
Contributor Author

nikhilmaheshwari24 commented Jul 30, 2024

PR Updated ✅ @realshuting

Copy link
Member

@realshuting realshuting left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@realshuting
Copy link
Member

Is #951 relevant?

@nikhilmaheshwari24
Copy link
Contributor Author

My bad. I have updated the comment.

@realshuting realshuting merged commit f2f2b24 into kyverno:main Jul 30, 2024
6 checks passed
Copy link

welcome bot commented Jul 30, 2024

Congratulations! 🎉

Great job merging your first Pull Request here! How awesome! If you are new to this project, feel free to join our Slack community.

200w

@chipzoller
Copy link
Contributor

@realshuting, @nikhilmaheshwari24: changes should not have been made here as these Markdown files are rendered from the contents of the policy library (kyverno/policies). We run a rendering program to then convert to Markdown. These changes will be wiped unless the corresponding policies in the library are updated.

@chipzoller
Copy link
Contributor

The changes made are also incorrect as without conditionals it will cause all Pods to fail if they don't have initContainers or ephemeralContainers.

@nikhilmaheshwari24
Copy link
Contributor Author

changes should not have been made here as these Markdown files are rendered from the contents of the policy library (kyverno/policies). We run a rendering program to then convert to Markdown. These changes will be wiped unless the corresponding policies in the library are updated.

@chipzoller,

  • Should I revert the PR changes and raise a new PR in kyverno/policies to update the corresponding policy in the library?
  • Or should I fix the changes here as well and raise a new PR in kyverno/policies to update the corresponding policy in the library?

Policy Path - https://github.com/kyverno/policies/blob/main/best-practices/require-pod-requests-limits/require-pod-requests-limits.yaml

@chipzoller
Copy link
Contributor

I'd just open the PR on kyverno/policies. We'll render policies for the website and pick up those changes.

@nikhilmaheshwari24
Copy link
Contributor Author

If you haven't raised the PR, let me do it, I want to contribute.

@nikhilmaheshwari24 nikhilmaheshwari24 changed the title Enhance Resource Constraints Validation Policy to Include InitContainers Enhance Resource Constraints Validation Policy to Include InitContainers & EphermalContainers Jul 31, 2024
@realshuting
Copy link
Member

@realshuting, @nikhilmaheshwari24: changes should not have been made here as these Markdown files are rendered from the contents of the policy library (kyverno/policies). We run a rendering program to then convert to Markdown. These changes will be wiped unless the corresponding policies in the library are updated.

I was not aware of that, thanks for pointing it out.

@nikhilmaheshwari24 - let's open a PR in kyverno/policies with the revised policy, thanks.

@nikhilmaheshwari24
Copy link
Contributor Author

@realshuting - PR kyverno/policies#1103

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants